Skip to content

feat: Advanced Invoice Tests & Refactor - #2

Merged
maheshrijal merged 17 commits into
mainfrom
refactor/es-modules
Jan 2, 2026
Merged

maheshrijal merged 17 commits into
mainfrom
refactor/es-modules

Conversation

@maheshrijal

Copy link
Copy Markdown
Owner

Changes

  • Refactored codebase to use ES Modules.
  • Added comprehensive Playwright tests in `tests/advanced-features.spec.js` covering:
    • Tax and discount calculations.
    • Due date handling.
    • Currency formatting.
    • PDF content verification.
  • Downgraded `pdf-parse` to v1.1.1 to fix API incompatibility.
  • Verified CI compatibility with GitHub Actions.

Verification

  • All tests passing locally.

- Reduce table border thickness from 1px to 0.5px
- Reduce cell padding and margins for more compact rows
- Remove table indent columns for proper margin alignment
- Align totals section with table right edge
@greptile-apps

greptile-apps Bot commented Jan 2, 2026 •

Copy link
Copy Markdown

Greptile Summary

This PR successfully refactors the codebase from a monolithic script.js to a modular ES Modules architecture, improving maintainability and testability. The refactor splits functionality into logical modules (calculations.js, form.js, pdf.js, invoice.js, etc.) while maintaining backward compatibility through legacy data migration.

Key Changes:

  • Migrated to ES Modules with "type": "module" in package.json
  • Added comprehensive Playwright E2E tests covering invoice generation, PDF content validation, tax/discount calculations, due dates, and currency formatting
  • Downgraded pdf-parse to v1.1.1 to resolve API compatibility issues
  • Set up GitHub Actions CI workflow with proper test reporting and artifact retention
  • Restructured CSS into modular components for better organization

Code Quality:

  • Clean separation of concerns with well-defined module boundaries
  • Proper error handling in PDF generation and storage operations
  • Good test coverage with both basic functionality and advanced feature tests
  • Tests use proper async/await patterns and dialog handling

The refactor maintains all existing functionality while significantly improving code organization and adding automated testing infrastructure.

Confidence Score: 5/5

  • This PR is safe to merge with high confidence - it's a well-executed refactoring with comprehensive test coverage
  • The refactor follows best practices with modular architecture, maintains backward compatibility, and adds robust test coverage. All previously flagged issues were already addressed in commit history. The ES module migration is clean with proper exports/imports, and the test suite validates critical functionality including PDF generation and calculations.
  • No files require special attention

Important Files Changed

Filename Overview
package.json Added ES module configuration with Playwright test scripts and pdf-parse v1.1.1 for compatibility
tests/advanced-features.spec.js Comprehensive test coverage for tax/discount calculations, due dates, currency formatting, and PDF content verification
tests/rebill.spec.js Core E2E tests for invoice generation, PDF download, validation, and history management with proper async handling
src/js/main.js Clean ES module entry point with proper event handlers, state management, and window global exports
src/js/modules/form.js Form validation and data sync with legacy data migration support; validation checks ANY valid item rather than ALL items
src/js/modules/pdf.js Well-structured PDF generation with proper formatting, conditional totals display, and Web Share API fallback
playwright.config.js Properly configured for CI with parallel tests, retries, and webServer auto-start

Sequence Diagram

sequenceDiagram
    participant User
    participant main.js
    participant form.js
    participant items.js
    participant calculations.js
    participant invoice.js
    participant pdf.js
    participant storage.js

    User->>main.js: Fill form & click "Generate Bill"
    main.js->>form.js: syncFromForm()
    form.js->>items.js: collectItemsFromForm()
    items.js-->>form.js: items array
    form.js->>calculations.js: calculateTotals(items, taxRate, discount)
    calculations.js-->>form.js: totals object
    form.js->>form.js: validateData()
    alt Valid Data
        form.js->>storage.js: saveDraft(data)
        storage.js-->>form.js: saved
        form.js->>invoice.js: renderInvoice(data)
        invoice.js-->>User: Display preview
        User->>main.js: Click "Download PDF"
        main.js->>pdf.js: downloadPDF()
        pdf.js->>form.js: getDataFromForm()
        form.js-->>pdf.js: invoice data
        pdf.js->>pdf.js: generateDocumentDefinition(data)
        pdf.js->>pdf.js: pdfMake.createPdf().download()
        pdf.js-->>User: PDF file downloaded
    else Invalid Data
        form.js-->>User: Alert "fill required fields"
    end
Loading

@maheshrijal

Copy link
Copy Markdown
Owner Author

@greptileai would it be possible to split css as well?

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at the current codebase structure, I can see you've already done an excellent job refactoring JavaScript into ES modules. The CSS could definitely benefit from a similar modular approach to improve maintainability and organization.

Here's how you could split the CSS into logical modules:

/* Create a new directory structure: */
src/
├── css/
│   ├── main.css              /* Main entry point with imports */
│   ├── base/
│   │   ├── reset.css         /* CSS reset and base styles */
│   │   ├── variables.css     /* CSS custom properties */
│   │   └── typography.css    /* Font definitions and text styles */
│   ├── components/
│   │   ├── buttons.css       /* Button styles */
│   │   ├── forms.css         /* Form inputs and controls */
│   │   ├── history.css       /* History panel styles */
│   │   └── invoice.css       /* Invoice preview styles */
│   ├── layout/
│   │   ├── grid.css          /* Layout grid system */
│   │   └── responsive.css    /* Media queries and responsive styles */
│   └── utilities/
│       ├── animations.css    /* Keyframes and transitions */
│       └── helpers.css       /* Utility classes */

The main benefits would be:

  1. Better organization - Each component's styles in its own file
  2. Easier maintenance - Find and modify specific styles quickly
  3. Reduced conflicts - Isolated concerns reduce CSS specificity issues
  4. Better collaboration - Team members can work on different style modules
  5. Selective loading - Could potentially load only needed styles

Would you like me to show you how to implement this CSS modularization? I can start by extracting specific sections like the invoice styles or form components into separate files.

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Jan 2, 2026 •

Copy link
Copy Markdown

Deploying rebill with  Cloudflare Pages  Cloudflare Pages

Latest commit: 44235ce
Status: ✅  Deploy successful!
Preview URL: https://a3e8665c.rebill.pages.dev
Branch Preview URL: https://refactor-es-modules.rebill.pages.dev

View logs

@github-actions

github-actions Bot commented Jan 2, 2026 •

Copy link
Copy Markdown

Playwright test results

passed  48 passed

Details

stats  48 tests across 2 suites
duration  58.5 seconds
commit  44235ce

@maheshrijal

Copy link
Copy Markdown
Owner Author

@greptile

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR refactors the codebase to use ES Modules and adds comprehensive Playwright end-to-end tests for invoice generation functionality. The changes include splitting the application into modular components for storage, JSON I/O, and item management, along with extensive test coverage for basic and advanced features including PDF content verification.

Key changes:

  • New Playwright test suites covering invoice form, generation, PDF verification, history, JSON import/export, and advanced features (tax, discount, due dates, currency)
  • Modular JavaScript refactoring with new modules for storage, JSON I/O, and items management
  • CSS refactoring into separate animation utility file

Reviewed changes

Copilot reviewed 31 out of 36 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
tests/rebill.spec.js New E2E tests for core invoice functionality including form validation, generation, PDF download, history, and JSON operations
tests/advanced-features.spec.js New tests for advanced features like tax/discount calculations, due dates, currency formatting, and PDF content verification
tests/rebill.spec.js-snapshots/*.png Visual regression test snapshots for webkit and firefox browsers
src/js/modules/storage.js New module handling draft save/load with deep merge utility and prototype pollution protection
src/js/modules/json-io.js New module for JSON import/export functionality
src/js/modules/items.js New module for line items form management
src/css/utilities/animations.css Extracted animations CSS into separate utility file
styles.css Removed (likely split into modular CSS files)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/js/modules/form.js Resolved
Comment thread src/js/main.js Resolved
Comment thread src/js/main.js Resolved
Comment thread src/js/main.js Resolved
Comment thread src/js/modules/json-io.js Resolved
maheshrijal and others added 3 commits January 2, 2026 15:56
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot AI commented Jan 2, 2026

Copy link
Copy Markdown
Contributor

@maheshrijal I've opened a new pull request, #3, to work on those changes. Once the pull request is ready, I'll request review from you.

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

32 files reviewed, 3 comments

Edit Code Review Agent Settings | Greptile

Comment thread src/js/modules/history.js Resolved
Comment thread src/js/modules/history.js Resolved
Comment thread src/js/modules/pdf.js Resolved
* Initial plan

* Remove unused imports getDataFromForm and updateFormTotalsDisplay from main.js

Co-authored-by: maheshrijal <62394512+maheshrijal@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: maheshrijal <62394512+maheshrijal@users.noreply.github.com>
@maheshrijal

Copy link
Copy Markdown
Owner Author

@greptile

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 31 out of 36 changed files in this pull request and generated no new comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@maheshrijal

Copy link
Copy Markdown
Owner Author

@greptile

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 31 out of 36 changed files in this pull request and generated no new comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

32 files reviewed, 3 comments

Edit Code Review Agent Settings | Greptile

Comment thread src/js/modules/form.js Resolved
Comment thread tests/rebill.spec.js Resolved
Comment thread src/js/modules/items.js Resolved
@maheshrijal

Copy link
Copy Markdown
Owner Author

@greptile

@maheshrijal

Copy link
Copy Markdown
Owner Author

@greptile

@maheshrijal
maheshrijal merged commit bea17e1 into main Jan 2, 2026
3 checks passed
@maheshrijal
maheshrijal deleted the refactor/es-modules branch January 2, 2026 11:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants